Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds telemetry on number of DNS queries served, per-agent. #1537

Merged
merged 2 commits into from
Dec 22, 2015

Conversation

slackpad
Copy link
Contributor

Adds a counter increment for each successful DNS query (reverse or forward). We had a request to aggregate this per-agent so I suffixed with the node name. We haven't done metrics per node before, though we do a similar thing for services, and Raft metrics roll up per-IP, so there seemed like precedent.

@ryanuber
Copy link
Member

This seems reasonably useful. We should probably tack this into the deferred function in each handler, unless there is a reason we are trying to avoid logging the metric when doing recursion. Do you think it would also be worth it to log a metric for the duration since we have the timestamp handy? I could see that being useful for the recursion case to spot problems downstream.

@slackpad
Copy link
Contributor Author

@ryanuber good call. We get a "count" metric for free with the gauge so no need for two metrics.

slackpad added a commit that referenced this pull request Dec 22, 2015
Adds telemetry on number of DNS queries served, per-agent.
@slackpad slackpad merged commit d1b4408 into master Dec 22, 2015
@slackpad slackpad deleted the f-dns-counting branch December 22, 2015 02:26
@scalp42
Copy link
Contributor

scalp42 commented Dec 22, 2015

Still hoping we get to have metrics without the hostname in the actual metric name 😇

@slackpad
Copy link
Contributor Author

@scalp42 ah yeah this will make that a little worse temporarily. I don't think this'll make it into the next point release but I'll try to get #1241 in the release after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants